-
-
Notifications
You must be signed in to change notification settings - Fork 13
Add TypeScript template #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add TypeScript template #152
Conversation
|
OK, did some testing around the first-run experience once someone generates a package with this template. It's quite likely they'll have generated the package by invoking a command from package-generator, so I also replicated the steps that would've been done by that package instead of Because the package template specifies
That should prompt them to install dependencies and run I also tweaked the generated TS so that the user isn't met with a bunch of linter errors immediately. |
|
I think I meant to make this a draft, but can't find any evidence that it ever was a draft. However, now that I've done the manual testing I promised, pretend I just took this out of draft! |
|
As usual, the CI needs a major medical intervention because we committed the crime of relying on something. |
|
I did a CI fixup in #154. If you like, I can rebase this branch on top of that, or fix the merge conflict. I'm not super involved with TypeScript, but I'm inclined to believe you that this is a good way to set up [a template for] new TypeScript packages. (And it can always be improved later on down the line.) CI passes with updated tests, so that's nice. |
Yeah, feel free to update this PR! I'm deep in some other silly pursuit at the moment. |
…and also: * Remove a decaffeination idiom * Make JavaScript the default for `ppm init -p` instead of CoffeeScript * Stop advertising CoffeeScript support in the help text (without dropping support for it)
70bf357 to
42ce87e
Compare
It is done. 👍 |
|
OK, merged the big Node upgrade into this PR. This isn't something that absolutely must make it into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did I really not formally approve this yet? Okay, approved.
Would love a second pair of review eyes from someone who uses TypeScript, if that is forecoming, but... is it? I don't know.
Anyway, the intent is good, and nothing obvious jumps out to me as being wrong.
Moving to JS by default and demoting CoffeeScript heavily is the sort of thing that might be controversial if CoffeeScript weren't basically gone from popular use and largely superseded by recent JSisms.
FWIW: ppm init --package [name] --syntax typescript does work for me locally on this branch. So that's good!

Marking this as a draft for now because further testing needs to be done, but this is part of the work needed for pulsar-edit#1272.
This adds the ability to generate a TypeScript template by running
ppm init --package <package-name> --syntax typescript. We already allowed people to choose between JavaScript and CoffeeScript when generating a basic package template; this just adds TypeScript as another option.This TypeScript template does all of what's described in pulsar-edit#1272:
It also has our fork of
@types/atompresent indevDependenciessuch that type annotations will be present in one's IDE for the entireatomAPI. (Install pulsar-ide-typescript to enjoy this feature, of course.)This needs to be tested in practice before landing. I used this exact structure (albeit constructed by hand rather than generated) for pulsar-hover and it served me well. I've also verified that that package works equally well in mainline Pulsar as it does in PulsarNext.
What's left to test is running
ppm init -p foo -s typescriptand asserting that it generates a package that “works” out of the box (doesn't fail with syntax or build or load errors). This might be tricky; unlike other template types, this one's entry point is a generated file, so it won't be present after generation until the user runsnpm run installandnpm run build. The answer might be to make a stublib/index.jsas part of the template so that we can catch this and tell the user what to do.I also made very slight changes to how
ppm initworks:-s/--syntaxwas omitted, we were assuming the user wanted a CoffeeScript package. This absolutely should not be the default any longer. We can keep supporting it without much hassle, but we should stop advertising it in the help text, too. When-sis omitted, we now assume JavaScript. The specs have been updated accordingly.resultparameter ingenerateFromTemplatewas a classic relic of decaffeinated code. After verifying we weren't actually using that return value for anything, I removed it.